Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[GSoC2024] fix cvat-ui error when open a job with issue (#7600) #7760

Closed
wants to merge 3 commits into from
Closed

[GSoC2024] fix cvat-ui error when open a job with issue (#7600) #7760

wants to merge 3 commits into from

Conversation

umangapatel123
Copy link
Contributor

@umangapatel123 umangapatel123 commented Apr 12, 2024 •
edited
Loading

Fixed an issue of error in cvat-ui when a job with issue is opened

The canvas ready state is not updated when unmounting canvas
make changes to set canvas ready state to false when canvas is unmounted.

Motivation and context

Fixes #7600
This change is necessary because it will make sure that the canvas is ready before attaching the issue label to the canvas

How has this been tested?

Manually

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • [ ] I have updated the documentation accordingly
  • [ ] I have added tests to cover my changes
  • I have linked related issues (see GitHub docs)
  • [ ] I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

@umangapatel123
[GSoC2024] fix cvat-ui error when open a job with issue (cvat-ai#7600)
a137ce9 
Fixed an issue of error in cvat-ui when a job with issue is opened
Fixes cvat-ai#7600
canvas ready state is not updated when unmounting canvas
make changes to set canvas ready state to false when canvas is unmounted
@umangapatel123
Copy link
Contributor Author

@nmanovic @bsekachev can you please review this PR?

@bsekachev
Copy link
Member

Hello,

Did you try to use existing action AnnotationActionTypes.CLOSE_JOB?
And set canvasReady to false when it is dispatched?

@umangapatel123
Copy link
Contributor Author

@bsekachev, No sir. I haven't tried doing that I have just created a new action AnnotationActionTypes.UPDATE_CANVAS_READY and dispatched the action when canvas unmountes to set the canvasReady to false.

@umangapatel123
Copy link
Contributor Author

also the AnnotationActionTypes.CLOSE_JOB is not dispatched when you go directly from the annotation page to jobs page

@bsekachev
Copy link
Member

We do not need extra action, because CLOSE_JOB already exists and used to handle canvas:
image

also the AnnotationActionTypes.CLOSE_JOB is not dispatched when you go directly from the annotation page to jobs page

There is code in cvat-ui/src/components/annotation-page/annotation-page.tsx

            if (!history.location.pathname.includes('/jobs')) {
                closeJob();
            }

written 4 years ago. Jobs page has been added much later. Perhaps we need to reconsider this fragment. Unfortunately, I do not remember why we avoid closing the job when pathname contains jobs. Probably to avoid removing job instance with annotations, etc. from the client, when component somehow get's unmounted. And we may write a better condition to handle the case.

What is more, the fact that closeJob() is one more bug (I suspect that it may lead to memory leaking).

@bsekachev
Copy link
Member

Hello @umangapatel123
Do you have any plans regarding the pull request?

@bsekachev
Copy link
Member

Please, let us know if you are ready to continue.

@bsekachev bsekachev closed this Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bsekachev bsekachev Awaiting requested review from bsekachev bsekachev is a code owner

@nmanovic nmanovic Awaiting requested review from nmanovic nmanovic is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cvat-ui error when open a job with issues
2 participants
@umangapatel123 @bsekachev